Skip to content

Add support for 1x2 layout#18

Merged
timcogan merged 1 commit intomasterfrom
feat/1x2
Feb 26, 2026
Merged

Add support for 1x2 layout#18
timcogan merged 1 commit intomasterfrom
feat/1x2

Conversation

@timcogan
Copy link
Owner

@timcogan timcogan commented Feb 25, 2026

Summary by CodeRabbit

  • New Features

    • Expanded multi-view support to flexible layouts: 1x1, 1x2, and 2x2 configurations
    • Dynamic UI grid rendering that adapts to actual image counts
    • Improved streaming and loading behavior for multi-view groups
  • Bug Fixes

    • More accurate active-group detection and completion handling for 2- and 4-image groups
    • Clearer multi-view status and error messages
  • Tests

    • Updated test coverage to validate 1-, 2- and 4-image multi-view scenarios

@coderabbitai
Copy link

coderabbitai bot commented Feb 25, 2026

Warning

Rate limit exceeded

@timcogan has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 15 minutes and 58 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between 901578e and aa2f073.

📒 Files selected for processing (3)
  • README.md
  • src/app.rs
  • src/dicomweb.rs
📝 Walkthrough

Walkthrough

Generalizes mammography group handling from fixed 4-image sets to support 1-, 2-, or 4-image multi-view groups. Adds helpers for validating group sizes, computing grid dimensions and labels; updates DICOMWeb grouping/validation, UI grid rendering, streaming logic, status messages, history/preload paths, and tests.

Changes

Cohort / File(s) Summary
App multi-view logic
src/app.rs
Adds is_supported_multi_view_group_size(), multi_view_grid_dimensions(), multi_view_layout_label() and replaces hard-coded 2x2/4-image assumptions with dynamic 1/2/4 handling across group checks, UI grid rendering, status messages, history/preload, and tests.
DICOMWeb group validation
src/dicomweb.rs
Expands validation to accept 1, 2, or 4 series UIDs/instances per group; adds early return path for 2-instance resolved sets in reduce_series_instances() and updates error messages to reflect allowed counts.
Tests & messaging
tests/..., src/... status strings
Updates test expectations and user-facing status/error messages to reference 1x1, 1x2, and 2x2 multi-view terminology instead of fixed 2x2 wording.

Sequence Diagram(s)

mermaid
sequenceDiagram
participant UI as UI / Renderer
participant App as DicomViewerApp
participant DWeb as DICOMWeb Client
UI->>App: request/load multi-view group (1/2/4)
App->>DWeb: query series/group instances
DWeb-->>App: return 1/2/4 instances
App->>App: validate with is_supported_multi_view_group_size()
alt valid group
App->>UI: compute grid (multi_view_grid_dimensions), render slots
UI-->>App: request instance streaming per slot
App->>DWeb: stream instances
DWeb-->>App: stream chunks / complete
App->>UI: update slot(s) / status messages
else invalid group
App->>UI: show error message (allowed: 1,2,4)
end

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

Poem

🐰 From four we now hop to one or two,
Small grids that fit the views we choose anew.
Helpers count and label, rows and columns play,
A rabbit cheers: flexible mammo day! 🥕

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 4.17% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: adding support for 1x2 layout alongside existing 2x2 support by generalizing multi-view group handling to support 2- or 4-image configurations.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/1x2

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/app.rs`:
- Around line 261-264: The clippy "redundant closure" warning comes from using
closures like |count| Self::is_supported_multi_view_group_size(count) when
checking dicomweb_active_group_expected; replace those closures with a direct
function pointer reference (Self::is_supported_multi_view_group_size) in all
three occurrences (the call sites around dicomweb_active_group_expected /
dicomweb_active_path_receiver and the other two locations referenced) so the
Option::is_some_and uses the function reference instead of the unnecessary
closure.

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7953d7b and f507a18.

📒 Files selected for processing (2)
  • src/app.rs
  • src/dicomweb.rs

@timcogan timcogan merged commit 12460e4 into master Feb 26, 2026
7 checks passed
@timcogan timcogan deleted the feat/1x2 branch February 26, 2026 12:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant